[Woo POS][Local Catalog] Loading screen for missing catalog when launching POS #16272

staskus · 2025-10-23T09:04:27Z

Description

To show a loading screen for the missing catalog, I implemented a mechanism to track the progress of ongoing sync within POSCatalogSyncCoordinator:

fullSyncStateStream to observe ongoing sync state: notStarted, started, completed, error...
lastFullSyncState to check a cached last state if it was completed before

Created POSCatalogLoadingView to be displayed when the catalog loads. Added matched geometry between a loading view and a catalog view, since it's impossible not to show a normal loading view, at least for a split second. And the progress view position doesn't fall between the two views since the progress view is centered together with the text in the catalog view.
Expanded PointOfSaleDashboardViewHelper and PointOfSaleDashboardView.ViewState to show catalogSync state
Removed an unnecessary initial delay for syncing the inventory.

Error handling will be done separately: WOOMOB-1565

It also made me think of creating a task for caching the eligibility check, and if the POS becomes ineligible, just showing a full-screen overlay. It's an unlikely scenario, and it's better to show POS immediately to give a fast execution impression.

Steps to reproduce

Fresh login

Fresh login to the Woo app
Open the POS immediately after the tab shows up
Confirm syncing state shows up
Confirm the product list loads once the syncing is done
Close POS/the app
Reopen and select the POS tab
Confirm syncing is no longer shown

Switch sites

Switch to another POS eligible site
Open the POS immediately after the tab shows up
Confirm syncing state shows up
Tap "Exit POS"
Wait for sync to complete (observe logs)
Open POS
Confirm the syncing state is no longer shown, and POS opens

Testing information

Tested on iPad Air 26 simulator.

Screenshots

catalog.sync.full.mov

I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

wpmobilebot · 2025-10-23T09:14:20Z

📲 You can test the changes from this Pull Request in WooCommerce iOS Prototype by scanning the QR code below to install the corresponding build.

	App Name	WooCommerce iOS Prototype
	Build Number	`pr16272-6f0230e`
	Version	`23.5`
	Bundle ID	`com.automattic.alpha.woocommerce`
	Commit	`6f0230e`
	Installation URL	1kebak5i64dk0

Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

joshheald

It also made me think of creating a task for caching the eligibility check, and if the POS becomes ineligible, just showing a full-screen overlay. It's an unlikely scenario, and it's better to show POS immediately to give a fast execution impression.

Eligibility for POS, or for local catalog? For the latter, see my latest PR, it moves towards that and has an element of caching. We fall back to the network based approach if they're ineligible. #16276

I've had some initial issues when testing this:

Catalogs over 1000 items attempt to sync, and never leave the syncing state when it fails. For these sites, we shouldn't even attempt the sync, but this seems to be solved if you merge the PR I mentioned above.
Syncing catalog can complete, but not transition to the item list, it just spins forever. This happened when I tried to sync an eligible store after opening POS for a store with too many products for local catalog, so I think there may be some state leaked?

joshheald

This works well, at least when merged with #16276, but I've left some questions/thoughts in line about another way we could approach it which might be easier

joshheald · 2025-10-23T14:35:45Z

...Sources/PointOfSale/Presentation/CardReaderConnection/UI States/PointOfSaleLoadingView.swift

                Spacer()
                ProgressView()
                    .progressViewStyle(POSProgressViewStyle())
+                    .matchedGeometryEffect(id: animation.progressTransitionId, in: animation.namespace, properties: .position)


What about using this existing LoadingView for local catalog syncs as well, rather than making a new view? That way we could set text fields to show, but keep using the same loading indicator and avoid visual glitches...

I think it would be a case of adding associated values to the containerState's loading enum case, perhaps?

I think that wouldn;t help with visual glitches.

We transition from PointOfSaleLoadingView within EntryPoint to PointOfSaleLoadingView within Dashboard, and when we add the text, the progress view moves up since, according to the designs, the whole container is centered, not the progress view. I tried centering only the progress view, but that doesn't look right.

But yes, we could keep one view that does more. We would still need to pass matched geometry from the EntryPoint to the Dashboard.

joshheald · 2025-10-23T14:38:15Z

Modules/Sources/PointOfSale/Models/PointOfSaleAggregateModel.swift

+        Task { @MainActor [weak self] in
+            guard let self else { return }
+
+            catalogSyncState = await coordinator.lastFullSyncState(for: siteID)
+
+            for await state in coordinator.fullSyncStateStream {
+                catalogSyncState = state
+            }
+        }


I think this task might never end, and therefore be a memory leak. It doesn't capture self, so no retain cycle, but we'll have the task hanging out for each time we leave and enter POS. Perhaps we should keep hold of it, and cancel it when we deinit?

Yes, it doesn't end since we want an endless stream. Good point on a memory leak. Need to be careful with this 🤔

AsyncStream may not be what I want as a pattern or at least not the way I implemented it. Canceling a task terminates the stream automatically, which has to be recreated. Otherwise, it doesn't emit values anymore. I need a pattern that Observation or Combine implements for continuous value streaming to multiple listeners.

I think it would be fine to just keep hold of the task in a property, and cancel/finish when we deinit the model, wouldn't it?

Yes, canceling the Task terminates the stream. I think I found a better way just to use Observation. I will push the changes soon.

Why does that matter, on deinit? We want it to end then, surely.

We won't use AsyncStream anymore but that mattered because when AsyncStream is terminated, there was no mechanism to recreate it. It wouldn't emit any new results after opening POS for the second time.

joshheald · 2025-10-23T14:42:49Z

Modules/Sources/Yosemite/Tools/POS/POSCatalogSyncCoordinator.swift

+        case .syncStarted(let id, _):
+            id
+        case .syncCompleted(let id):
+            id
+        case .syncFailed(let id, _):
+            id
+        case .syncNeverDone(let id):
+            id


Suggested change

case .syncStarted(let id, _):

id

case .syncCompleted(let id):

id

case .syncFailed(let id, _):

id

case .syncNeverDone(let id):

id

case .syncStarted(let id, _), .syncCompleted(let id), .syncFailed(let id, _), .syncNeverDone(let id):

id

joshheald · 2025-10-23T14:43:42Z

Modules/Sources/Yosemite/Tools/POS/POSCatalogSyncCoordinator.swift

+            return lhsSiteID == rhsSiteID && lhsInitial == rhsInitial
+        case (.syncCompleted(let lhsSiteID), .syncCompleted(let rhsSiteID)):
+            return lhsSiteID == rhsSiteID
+        case (.syncFailed(let lhsSiteID, _), .syncFailed(let rhsSiteID, _)):


Are we OK with two failures that have different errors being considered equal?

Good question. We shouldn't have to error transition, since we would have a loading state in between, but I guess it would be more correct to check error equality as well 🤔

joshheald · 2025-10-23T14:47:26Z

Modules/Sources/PointOfSale/ViewHelpers/PointOfSaleDashboardViewHelper.swift

+        if let syncState = catalogSyncState {
+            switch syncState {
+            case .syncStarted(_, true):
+                return .catalogSyncing
+            case .syncNeverDone:
+                return .catalogSyncing
+            case .syncStarted(_, false):
+                // Non-initial sync, continue to other checks
+                break
+            case .syncCompleted:
+                // Continue to other checks
+                break
+            case .syncFailed:
+                // TODO: WOOMOB-1565
+                return .error(PointOfSaleErrorState.errorOnLoadingOrders())
+            }
+        }


Perhaps we'd avoid the need for this (and some of the async stream stuff) if the GRDBItemsController handled this and set the container state to .loading(.catalogSyncing)? I know it's a big change, and I could be missing some fundamental reason why it won't work or would be a bad way to go...

Fair point. I mean, we would still need changes within the coordinator, since it's the one that does the syncing. Syncing can start at any time, so we need a mechanism that keeps and streams the syncing state.

In this case, I imagine the difference would be replacing aggregateModel+viewHelper with grdbItemsController, but other things would be mostly the same. Correct me if I'm wrong.

and some of the async stream stuff

Do you see how it can be avoided if we implemented the logic within the controller?

Do you see how it can be avoided if we implemented the logic within the controller?

No, thinking about it more we'd still need something like the stream, which should definitely still be in the coordinator, so that we can show the current sync if there's one already ongoing.

We'd be reading that stream in the GRDBItemsController, and setting the container state there too. I didn't really know that we used this helper, to be honest... it's probably fine in either place.

staskus · 2025-10-23T15:28:26Z

@joshheald Thanks for testing! Yes, I admit I found these tasks with combining sync states, GRDB observation, and SwiftUI more complicated and time-consuming than I anticipated. There are a lot of little details and even after thorough work and testing there are issues that I don't like to see...

I've had some initial issues when testing this:

Eligibility for POS, or for local catalog?

Eligibility for POS. I would imagine that with a loaded POS Catalog, we could launch straight into the POS without any loading screen. But that's a topic for future improvements.

Catalogs over 1000 items attempt to sync, and never leave the syncing state when it fails. For these sites, we shouldn't even attempt the sync, but this seems to be solved if you merge the PR I mentioned above.

Yes, I haven't addressed these limitations in this PR.

Syncing catalog can complete, but not transition to the item list, it just spins forever. This happened when I tried to sync an eligible store after opening POS for a store with too many products for local catalog, so I think there may be some state leaked?

When developing, I tested multiple times with various stores, but I haven't noticed this case. I will try to understand what could be happening here.

…inator Introduces a fullSyncStateStream and state cache to POSCatalogSyncCoordinator, allowing consumers to observe full sync state changes. Adds a POSCatalogSyncState enum to represent sync progress, completion, failure, and never-done states. Refactors sync logic to emit state updates and provides a method to query the last known sync state for a site.

Generic loading view appears briefly why POS model is loading. It's hard to get away from that. Synchronizing animations between both loading views allows to transition between both loading states without any noticeable transition issues.

staskus · 2025-10-24T13:01:04Z

@joshheald thanks for the comments. I think I managed to address them with quite substantial changes:

Moved from using AsyncStream to @Observable for observing catalog sync state, which removed a category of issues and complications
Using coordinator.fullSyncState in PointOfSaleObservableItemsController, which automatically updates the container state if fullSyncState changes, not requiring any other observation logic
Merged POSCatalogLoadingView and PointOfSaleLoadingView , removed matchedGeometry and passing namespace

catalog.syncing.mov

Assigning to you, to merge after woomob-1098-woo-poslocal-catalog-add-incremental-sync-triggers-pull-to

Thank you again for all the help! 🙇

dangermattic · 2025-10-30T14:12:55Z

	1 Warning
⚠️	This PR is assigned to the milestone 23.6. This milestone is due in less than 2 days. Please make sure to get it merged by then or assign it to a milestone with a later deadline.

Generated by 🚫 Danger

…reen-for-missing-catalog-when

staskus added this to the 23.6 milestone Oct 23, 2025

staskus requested a review from joshheald October 23, 2025 09:04

staskus added type: task An internally driven task. feature: POS labels Oct 23, 2025

joshheald self-assigned this Oct 23, 2025

joshheald reviewed Oct 23, 2025

View reviewed changes

joshheald approved these changes Oct 23, 2025

View reviewed changes

staskus force-pushed the woomob-1101-woo-poslocal-catalog-loading-screen-for-missing-catalog-when branch from 58e12ce to 45955b3 Compare October 23, 2025 16:15

staskus added 17 commits October 24, 2025 10:57

Create posBodySmallBold with underline syle

6f9b492

Update existing posBodySmallBold usage

27855e0

Create POSCatalogLoadingView to show the syncing catalog state

8a85286

Update POSCatalogSyncCoordinatorTests

45fa0ec

Remove initial delay for POSCatalogSync when the app launches

a6139be

Publish catalogSyncState from PointOfSaleAggregateModel

669ffc0

Check syncState in PointOfSaleDashboardViewHelper

debf355

Integrate POSCatalogLoadingView in PointOfSaleDashboardView

ff44181

Update PointOfSaleDashboardViewHelperTests

cde1ba7

Add a TODO for an error state handling

5b3b8ae

Update POSCatalogSyncCoordinator.swift

acb40e7

Check error in syncFailed

90f2949

Observe catalog sync state using Observation model

0232f01

Move catalogSyncState checking logic to ObservableItemsController

3f3b654

Merge PointOfSaleLoading view and POSCatalogLoadingView

644c9ee

staskus force-pushed the woomob-1101-woo-poslocal-catalog-loading-screen-for-missing-catalog-when branch from 45955b3 to 644c9ee Compare October 24, 2025 12:57

Base automatically changed from woomob-1098-woo-poslocal-catalog-add-incremental-sync-triggers-pull-to to trunk October 30, 2025 14:12

joshheald added 3 commits October 30, 2025 14:31

Merge branch 'trunk' into woomob-1101-woo-poslocal-catalog-loading-sc…

e900ecb

…reen-for-missing-catalog-when

Prevent saving first sync date if it doesn’t happen

8c1fe2c

Start syncs after updating catalog eligibility

6f0230e

joshheald enabled auto-merge October 30, 2025 19:14

joshheald merged commit 8e0c018 into trunk Oct 30, 2025
14 checks passed

joshheald deleted the woomob-1101-woo-poslocal-catalog-loading-screen-for-missing-catalog-when branch October 30, 2025 19:23

[Woo POS][Local Catalog] Loading screen for missing catalog when launching POS #16272

[Woo POS][Local Catalog] Loading screen for missing catalog when launching POS #16272

Uh oh!

Conversation

staskus commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Steps to reproduce

Fresh login

Switch sites

Testing information

Screenshots

Uh oh!

wpmobilebot commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joshheald left a comment

Choose a reason for hiding this comment

Uh oh!

joshheald left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

staskus Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

staskus commented Oct 23, 2025

Uh oh!

staskus commented Oct 24, 2025

Uh oh!

dangermattic commented Oct 30, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

staskus commented Oct 23, 2025 •

edited

Loading

wpmobilebot commented Oct 23, 2025 •

edited

Loading

staskus Oct 23, 2025 •

edited

Loading